gh-86542: New C-APIs to simplify module attribute declaration #23286
Conversation
tiran
force-pushed
the
module_constants
branch
3 times, most recently
from
ee79bab to
ef08e94
Compare
tiran
changed the title
tiran
force-pushed
the
module_constants
branch
2 times, most recently
from
51767ca to
bcd0d45
Compare
tiran
force-pushed
the
module_constants
branch
2 times, most recently
from
ed6c1b3 to
6de854c
Compare
| long m_long; | ||
| unsigned long m_ulong; | ||
| double m_double; | ||
| PyObject* (*m_call)(PyObject *module); |
There was a problem hiding this comment.
I'm not sure how to make this member future-proof in term of stable ABI.
We should try to put a max_align_t inside, but this type requires C11. GCC defines it with:
typedef struct {
long long __max_align_ll __attribute__((__aligned__(__alignof__(long long))));
long double __max_align_ld __attribute__((__aligned__(__alignof__(long double))));
/* _Float128 is defined as a basic type, so max_align_t must be
sufficiently aligned for it. This code must work in C++, so we
use __float128 here; that is only available on some
architectures, but only on i386 is extra alignment needed for
__float128. */
#ifdef __i386__
__float128 __max_align_f128 __attribute__((__aligned__(__alignof(__float128))));
#endif
} max_align_t;
Maybe we can at least put long long and long double:
// Unused members added to make PyModuleConst_Def large enough
// to get a stable ABI and support future additions.
long long m_long_long;
long double m_long_double;
There was a problem hiding this comment.
This is the only issue I found here :)
The stable ABI doesn't have a good story around evolving structs, and I think we should design a general mechanism for that rather than try to future-proof individual structs.
To move this PR forward, could we exclude PyModule_AddConstants & co. from the limited API for the time being?
| PyObject *module, PyType_Spec *spec, PyObject *base); | ||
| PyAPI_FUNC(PyObject *) PyModule_AddNewException( | ||
| PyObject *module, const char *name, const char *doc, | ||
| PyObject *base, PyObject *dict); |
There was a problem hiding this comment.
Could you start by adding these two functions in a separated PR, to make this PR shorter?
There was a problem hiding this comment.
I don't really think that's necessary; the PR is not that big.
|
|
||
| Example:: | ||
|
|
||
| static PyObject* |
There was a problem hiding this comment.
nitpick: static PyObject * in here.
| @@ -15698,9 +15620,98 @@ posixmodule_exec(PyObject *m) | |||
| return 0; | |||
| } | |||
|
|
|||
| static PyModuleConst_Def posix_constants[] = { | |||
There was a problem hiding this comment.
How about using another sepeareted PR to do this converation operation?
|
This PR is stale because it has been open for 30 days with no activity. |
Signed-off-by: Christian Heimes <[email protected]>
Add helpers to create new type/exception and add it to the module dict in one call. Signed-off-by: Christian Heimes <[email protected]>
Signed-off-by: Christian Heimes <[email protected]>
This reverts commit 042e0999ed5ecf88de63de1140d968ab14745e3f.
| The values for *type* and the definition of the *value* union are | ||
| internal implementation details. Use any of the ``PyModuleConst_`` macros | ||
| to define entries. The array must be terminated by an entry with name | ||
| set to ``NULL``. | ||
|
|
||
| .. c:member:: const char *name | ||
|
|
||
| Attribute name. | ||
|
|
||
| .. c:member:: int type | ||
|
|
||
| Attribute type. | ||
|
|
||
| .. c:member:: void *value | ||
|
|
||
| Value of the module constant definition, whose meaning depends on | ||
| *type*. |
There was a problem hiding this comment.
| The values for *type* and the definition of the *value* union are | |
| internal implementation details. Use any of the ``PyModuleConst_`` macros | |
| to define entries. The array must be terminated by an entry with name | |
| set to ``NULL``. | |
| .. c:member:: const char *name | |
| Attribute name. | |
| .. c:member:: int type | |
| Attribute type. | |
| .. c:member:: void *value | |
| Value of the module constant definition, whose meaning depends on | |
| *type*. | |
| Definition of a module constant. | |
| The members of this struct are internal implementation details. | |
| To define entries, use only the ``PyModuleConst_`` macros below, | |
| and ``{NULL}`` to mark the end of the array. |
| } | ||
|
|
||
| PyObject * | ||
| PyModule_AddNewException(PyObject *module, const char *name, const char *doc, |
There was a problem hiding this comment.
These two functions need tests.
There was a problem hiding this comment.
Hm, I see these return borrowed references (held by the module object). That saves a DECREF if you don't need the new object, but it'll be problematic for HPy.
github-actions
bot
removed
the
stale
erlend-aasland
changed the title
|
I'll close this as it's not likely to get updated any time soon. |
refcounts.datSigned-off-by: Christian Heimes [email protected]
https://bugs.python.org/issue42376